-
Notifications
You must be signed in to change notification settings - Fork 5
DOCSP-46221: Atlas SDK for Go Monitoring & Logging scripts for Atlas Architecture Center #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -0,0 +1,69 @@ | |||
// :snippet-start: config-loader-function-full-example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can reuse instead env-var names from Terraform function:
func setDefaultValuesWithValidations(ctx context.Context, data *tfMongodbAtlasProviderModel, resp *provider.ConfigureResponse) tfMongodbAtlasProviderModel {
This way it is easier for users to transition between tools
) | ||
|
||
type Config struct { | ||
AtlasBaseURL string `json:"ATLAS_BASE_URL"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually, it is simpler to only use the MONGODB_ATLAS_BASE_URL
than using hostname, port, etc.
fb5d475
to
79ae2df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on this! It's a big deliverable in a new problem domain for our team, with some added complications re: snippets/copied app. I think you've done a great job of breaking it all down here.
I've got a handful of nitpicks, style questions, and minor suggestions, and one issue re: imports in the generated app copy that is preventing it from compiling unless I manually delete them. Marking this PR as "request changes" only because of that issue; everything else is a suggestion/nitpick/subjective.
generated-usage-examples/go/atlas-sdk-go/project-copy/internal/logs/fetch_test.go
Outdated
Show resolved
Hide resolved
go run cmd/get_logs/main.go | ||
``` | ||
|
||
## Set up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a Q - should this Set up
section also be in the public README? Seems it would be useful to give folks info about the service account prereq, and info about where/how to set env variables and config file information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re. this and following comment, I do have a version of this content in the public README draft, but I'll include it in a separate PR. I didn't want it to keep me from dragging this PR out even longer 😓
Co-authored-by: Dachary <dc@dacharycarey.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work with the updates. This is looking good. LGTM! 🚢
DOCSP-46221: Atlas SDK for Go Monitoring & Logging scripts for Atlas Architecture Center
Jira Ticket
DOCSP-46221
Description of Changes
IA Changes
/go
and/java
to support multiple sub-directoriesAtlas SDK for Go
/go/atlas-sdk-go
projectTODO (outside of this ticket)